Improve assistant management for more consistent behavior#14
Merged
Improve assistant management for more consistent behavior#14
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors assistant initialization to rely on the OpenAI API (removing local assistant ID persistence), adds assistants list and assistants clean CLI commands for remote assistant management, and cleans up related CLI entry points and docstrings.
- Renamed the
settings infocommand tosettings show - Introduced the
assistantscommand group withlistandcleansubcommands - Refactored
OpenAIClientto unify assistant/thread initialization, remove persisted assistant IDs, and added docstrings across client and handler classes
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/vecsync/cli/settings.py | Renamed info to show and updated registration in settings group |
| src/vecsync/cli/entry.py | Registered the new assistants command group |
| src/vecsync/cli/chat.py | Removed the --new-conversation flag and updated function signatures |
| src/vecsync/cli/assistants.py | Added assistants list and assistants clean commands |
| src/vecsync/chat/clients/openai.py | Refactored OpenAIClient, added connect/disconnect, cleaned up assistant persistence, and enriched docstrings |
| src/vecsync/chat/clients/base.py | Introduced Assistant Pydantic model |
| CHANGELOG.md | Documented the new assistant CLI commands and cleanup behavior |
Comments suppressed due to low confidence (2)
src/vecsync/cli/assistants.py:9
- [nitpick] Using
listas a function name shadows the built-inlist. Consider renaming tolist_assistantsor explicitly setting@click.command(name='list')to avoid confusion.
def list():
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The main change in this PR is to refactor the initialization of the assistant class such that we are more aligned with the remote state in OpenAI. Instead of locally managing the assistant ID, we now rely on OpenAPI. This should enforce at most a single assistant ID in the user account at any point in time.
Extra commands
assistants listandassistants cleanwere added to help manage the assistants.Test Plan
Locally ran through multiple scenarios of creating assistants, deleting assistants, and deleting settings to confirm behavior: